Skip to content

Fix unit preference changes for open documents - #189

Merged
10-X-eng merged 4 commits into
10-X-eng:mainfrom
Anteriousis:bugfix/issue-182-unit-preferences
Sep 9, 2026
Merged

Fix unit preference changes for open documents#189
10-X-eng merged 4 commits into
10-X-eng:mainfrom
Anteriousis:bugfix/issue-182-unit-preferences

Conversation

@Anteriousis

@Anteriousis Anteriousis commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

The unit preference change now updates the active document's unit system when project units are not being ignored, while preserving the view-only behavior when "Ignore project unit system" is enabled. A regression test covers changing units with an open document.

Verification

  • For code changes, a test failed before the implementation and passes afterward; for non-code changes, the PR explains why TDD does not apply.
  • The PR lists the exact build and test commands and their results.

The focused regression test failed against the pre-fix implementation and passes afterward:

  • pixi run cmake --build build/debug --target UnitSettings_Tests_run --parallel 8
  • pixi run ctest --test-dir build/debug -R UnitSettings_Tests_run --output-on-failure

Also verified the existing QuantitySpinBox_Tests_run test passes.

Issues

Fixes #182

Before and After Images

No visual changes; screenshots are not applicable.

codex added 2 commits September 6, 2026 10:52
@codex
Fix unit preference changes for open documents
511f75e
@codex
Preserve project units when applying unchanged preferences
fcc98da 
Only update the active document when the selected global unit preference changes. Preserve project units when applying unrelated preferences or disabling IgnoreProjectSchema. Expand Qt coverage to seven scenarios.

Red: the unchanged-preference and restore-project-units cases failed against the PR implementation before the fix. Green: both test executables pass after the fix.

Validation: pixi run cmake --build build/debug --target UnitSettings_Tests_run QuantitySpinBox_Tests_run --parallel 8 (passed)

Validation: pixi run ctest --test-dir build/debug -R "^(UnitSettings|QuantitySpinBox)_Tests_run$" --output-on-failure (2/2 passed)
@10-X-eng

10-X-eng commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Review: production change does not overlap the performance diff, but test registration conflicts with the new GUI tests. Quick validation: preserve both registrations, build/run UnitSettings_Tests_run and QuantitySpinBox_Tests_run against current main. Exercise an open document with IgnoreProjectSchema on/off and ensure changing unrelated preferences preserves document units. Hold until the conflict is resolved and those native tests pass.

@10-X-eng
10-X-eng enabled auto-merge (squash) September 9, 2026 23:27
@10-X-eng

10-X-eng commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Resolved test registration conflict while preserving DocumentBulkMutation and RenderMeshController performance tests. Focused native validation against the current native FreeCAD libraries:

python /tmp/vibecad-compile-unit-settings.py compiled PR UnitSettings.cpp, generated moc output, and linked a baseline test against existing FreeCADGui plus a changed test with the PR's DlgSettingsGeneral.cpp. Commands were derived from the native Ninja compilation database; no application libraries or active sanitizer sources were replaced.

QT_QPA_PLATFORM=offscreen XDG_CONFIG_HOME=/tmp/vibecad-unit-settings/config XDG_DATA_HOME=/tmp/vibecad-unit-settings/data /tmp/vibecad-unit-settings/UnitSettings-red: 7 passed, 2 failed (changing the open document's units), exit 2.
The same environment with UnitSettings-green: 9 passed, exit 0.
The same environment with /home/robit/vibecad/build/release/tests/QuantitySpinBox_Tests_run: 6 passed, exit 0.

These are focused native compile/link and GUI preference tests, not a fresh full application build. Auto-merge is subject to refreshed required checks.

@10-X-eng
10-X-eng merged commit 3b488a2 into 10-X-eng:main Sep 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

If you change units it breaks everything

3 participants

@Anteriousis @10-X-eng @codex